ENG-3044: Add search to Admin UI sidebar navigation#7723
Conversation
Add a search component to the sidebar nav that lets users quickly find and navigate to any page, tab, system, integration, or taxonomy type. Supports Cmd+K shortcut, Spotlight-style modal when collapsed, and full keyboard/screen-reader accessibility. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub. 2 Skipped Deployments
|
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ess collision The expanded nav search placeholder "Search" matched existing Cypress selectors (input[placeholder='Search']). Changed to "Go to..." to avoid the collision. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Replace eager prefetch of all systems/integrations with server-side search that fires after 2+ characters with a 300ms debounce. This scales to thousands of systems and integrations without loading them all into memory on page load. Static items (pages, tabs, taxonomy types) remain client-side filtered. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Override Ant Design CSS variables for modal content/body padding to ensure zero padding regardless of theme configuration. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Greptile SummaryThis PR adds a polished sidebar navigation search component to the Admin UI, supporting both an inline expanded mode and a Spotlight-style modal for the collapsed sidebar, with keyboard shortcuts, ARIA accessibility, server-side search for systems/integrations, and 40 tests. The implementation is well-structured, but there are a few issues to address before merging. Key findings:
Confidence Score: 3/5
Important Files Changed
Last reviewed commit: "Fix modal padding in..." |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Ant Design v5 wraps content in .ant-modal-container (not .ant-modal-content) which adds 20px 24px padding. Target both selectors for compatibility. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Use ul/li/span instead of div for modal results list (semantic HTML) - Split collapsed mode tests into NavSearch.collapsed.test.tsx - Add list-style reset for ul/li elements in results Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The backend LIKE search on 'ab' matches many systems containing those letters anywhere in the name. With size=10 relevant results like 'AB InBev' were pushed off the first page. Size=50 gives better coverage while the client-side filter still narrows to exact matches. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This reverts commit 6cfa3a9.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Move collapse/expand toggle from logo area to bottom bar footer. Logo is now a static display. In expanded mode the toggle is right-aligned; in collapsed mode it stacks below Help and Account. Restyle nav search input to blend into sidebar: minos fill with subtle neutral-700 border, neutral-800 on hover/focus, Cmd+K hint only visible on focus. Standardize bottom bar button styles across Help, Account, and collapse toggle with shared navBottomButton class. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Keep the logo clickable for users accustomed to that behavior. The footer button remains as a secondary affordance for discoverability. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
| /** Shared tab definitions used by both the tab bar and nav search. */ | ||
| export const ACCESS_CONTROL_TAB_ITEMS: NavConfigTab[] = [ | ||
| { title: "Summary", path: ACCESS_CONTROL_SUMMARY_ROUTE }, | ||
| { title: "Request log", path: ACCESS_CONTROL_REQUEST_LOG_ROUTE }, | ||
| ]; | ||
|
|
There was a problem hiding this comment.
I extracted the tab definitions to a const so we prevent drift as we add new tabs. The nav search uses these consts.
There was a problem hiding this comment.
I like where you're going.
Not sure if in scope, but I think our method of keeping track of paths is flawed.
If we stored paths as a record or enum, we could enforce uniqueness and also define the path of NavConfigTab as being a strict value from that record/enum so that we can make sure that urls aren't missed.
There was a problem hiding this comment.
Agreed, a centralized path record/enum would be a nice improvement for enforcing uniqueness. I'll note it as a follow-up since it touches more than just the search feature.
speaker-ender
left a comment
speaker-ender
left a comment
There was a problem hiding this comment.
Overall looks good and very cool!
Some small nits, comments, and questions.
| /** Shared tab definitions used by both the tab bar and nav search. */ | ||
| export const ACCESS_CONTROL_TAB_ITEMS: NavConfigTab[] = [ | ||
| { title: "Summary", path: ACCESS_CONTROL_SUMMARY_ROUTE }, | ||
| { title: "Request log", path: ACCESS_CONTROL_REQUEST_LOG_ROUTE }, | ||
| ]; | ||
|
|
There was a problem hiding this comment.
I like where you're going.
Not sure if in scope, but I think our method of keeping track of paths is flawed.
If we stored paths as a record or enum, we could enforce uniqueness and also define the path of NavConfigTab as being a strict value from that record/enum so that we can make sure that urls aren't missed.
| }, | ||
| ]; | ||
| const menuItems = ACCESS_CONTROL_TAB_ITEMS.map((tab) => ({ | ||
| key: tab.path === ACCESS_CONTROL_SUMMARY_ROUTE ? "summary" : "request-log", |
There was a problem hiding this comment.
I think the key can just be the path if it is unique
There was a problem hiding this comment.
Done, switched to using tab.path as the key and simplified the component.
| className={`inline-flex cursor-pointer p-0 ${styles.collapseToggle}`} | ||
| className={styles.logoToggle} |
There was a problem hiding this comment.
I believe we do prefer to use tailwind classes over css module styles when possible.
I think this is a pattern in this PR.
There was a problem hiding this comment.
Converted the layout wrappers and the collapsed button to Tailwind classes. The remaining SCSS covers Ant component overrides that require :global() selectors (modal, input, dropdown styling), which can't be expressed in Tailwind.
| const handleSelect = useCallback( | ||
| (key: string) => { | ||
| const path = keyToPath.get(key) ?? key; | ||
| justSelectedRef.current = true; | ||
| router.push(path); | ||
| setOpen(false); | ||
| setSearchValue(""); | ||
| inputRef.current?.blur(); | ||
| modalInputRef.current?.blur(); | ||
| // Reset the guard after the event loop settles | ||
| setTimeout(() => { | ||
| justSelectedRef.current = false; | ||
| }, 0); | ||
| }, | ||
| [router, keyToPath], | ||
| ); |
There was a problem hiding this comment.
Not sure if fully possible or makes sense for certain results but it would be nice if the routing was handled by a link component rather than a callback for simplicity.
There was a problem hiding this comment.
Switched the modal result items to NextLink in the new NavSearchModal.tsx. The expanded mode still uses a callback since Ant's AutoComplete controls rendering via onSelect.
| width={480} | ||
| style={MODAL_POSITION} | ||
| className={styles.searchModal} | ||
| destroyOnClose |
There was a problem hiding this comment.
If the inner content handled some of the estate, I think the destroyOnClose would handle some of state resetting without having to do it manually + using a callback.
There was a problem hiding this comment.
destroyOnClose handles the modal's internal DOM cleanup, but searchValue state lives in the parent component (needed to drive the input and filtered results), so we still need to clear it explicitly on close. With the component split, this is cleaner since handleClose in NavSearchModal just calls setOpen(false) and setSearchValue("").
- Split NavSearch into NavSearchExpanded + NavSearchModal components - Extract NavSearchResultItem as separate component - Use NextLink for modal result items instead of router.push callback - Switch to react-hotkeys-hook for Cmd+K/Ctrl+K shortcut - Rewrite grouping/item construction with reduce/flatMap (no mutations) - Use pluralize utility for result count announcement - Use path as key in AccessControlTabs - Convert layout wrappers to Tailwind, keep Ant overrides in SCSS - Fix aria-controls to always reference the listbox ID Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
speaker-ender
left a comment
speaker-ender
left a comment
There was a problem hiding this comment.
Left a comment about tests that I've seen as a re-occurring theme in PRs.
Otherwise looks good!
| // Mock fidesui to avoid jsdom incompatibilities with Ant Design components. | ||
| // NavSearch imports: AutoComplete, Icons, Input, InputRef, Modal | ||
| jest.mock("fidesui", () => { |
There was a problem hiding this comment.
Just calling this out as something to investigate and solve in the future as this isn't best practice to mock things like this.
It's codifying functionality that might have no bearing on reality and can cause false positives and negatives in tests.
Bad tests can be worse than no tests at all.
There was a problem hiding this comment.
I'll look into this as part of a separate ticket https://ethyca.atlassian.net/browse/ENG-3161
# Conflicts: # clients/admin-ui/src/features/access-control/AccessControlTabs.tsx
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
These icons are used in MainSideNav.tsx for the nav collapse toggle but were missing from the curated carbon.ts export list introduced in #7745, causing typecheck failures in the merge queue. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2 participants
Ticket ENG-3044
Description Of Changes
nav-search-demo.mp4
Add a search component to the Admin UI sidebar navigation that lets users quickly find and navigate to any page, tab, system, integration, or taxonomy type. In expanded mode, it renders as an inline autocomplete input in the sidebar. In collapsed mode, it opens a centered Spotlight-style modal. Supports Cmd+K / Ctrl+K keyboard shortcut, full arrow-key navigation, and screen-reader accessibility (ARIA combobox pattern with live region announcements).
Searchable items include:
Sub-items only appear when the search query matches them, keeping the default dropdown compact. Tab definitions are co-located with their source components and imported into nav-config to prevent drift.
Performance: Systems and integrations use server-side search with a 200ms debounce, firing only after 2+ characters are typed. This scales to thousands of records without prefetching everything into memory. Static items (pages, tabs, taxonomy types) remain client-side filtered for instant results.
Code Changes
clients/admin-ui/src/features/common/nav/NavSearch.tsx- New search component with expanded (autocomplete) and collapsed (modal) modesclients/admin-ui/src/features/common/nav/NavSearch.module.scss- Styles for both modes, dark sidebar theme, Spotlight modalclients/admin-ui/src/features/common/nav/useNavSearchItems.ts- Hook that combines static nav items with server-side search for systems and integrationsclients/admin-ui/src/features/common/nav/nav-config.tsx- AddedNavConfigTabinterface andtabsfield to route configclients/admin-ui/src/features/common/nav/MainSideNav.tsx- Integrated NavSearch between logo and menuclients/admin-ui/src/features/access-control/AccessControlTabs.tsx- Exported tab definitions for searchclients/admin-ui/src/features/common/NotificationTabs.tsx- Exported tab definitions for search; renamed Templates to Messaging templatesclients/admin-ui/src/features/privacy-requests/hooks/usePrivacyRequestTabs.ts- Exported tab definitions for searchclients/admin-ui/__tests__/features/common/nav/NavSearch.test.tsx- 40 tests covering both modes, filtering, keyboard shortcuts, navigation, tabs, duplicate paths, dynamic items, and accessibilitySteps to Confirm
Pre-Merge Checklist
CHANGELOG.mdupdated